Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove sources from schema and consolidate its URL field into caseReference #372

Merged
merged 9 commits into from
Jun 30, 2020

Conversation

khmoran
Copy link
Contributor

@khmoran khmoran commented Jun 29, 2020

Sorry it's so huge :\ I tried to break commits up somewhat logically

Note: I made caseReference required, as well as caseReference.sourceUrl

@khmoran
Copy link
Contributor Author

khmoran commented Jun 29, 2020

@khmoran khmoran requested a review from axmb June 30, 2020 12:58
Copy link
Contributor

@axmb axmb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done!


Returns:
None: When the input is empty.
Dict[str, str]: When the input is nonempty. The dictionary is in the
format:
{
'url': str,
'other': str
'sourceUrl': str,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Granted there's no real sourceId for this data, but it's a little odd that we're inserting data that disagrees with our Mongoose validation rules. Is it worth filling the ID with a hash of the URL (or perhaps some portion of it)?

Mostly related: should we add a required stanza(s) in our json schema?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Granted there's no real sourceId for this data, but it's a little odd that we're inserting data that disagrees with our Mongoose validation rules. Is it worth filling the ID with a hash of the URL (or perhaps some portion of it)?

I guess I was hoping we'd be able to backfill some of these ids, after we've added some data to the sources collection -- does that sound reasonable, and if reasonable, worth it?

Mostly related: should we add a required stanza(s) in our json schema?

unfortunately, there are a few existing cases that do not have a source URL, so they would fail to validate & insert into the db :( I can look into getting those fixed upstream though and then we can make it required

verification/curator-service/ui/src/components/Case.tsx Outdated Show resolved Hide resolved
@khmoran khmoran merged commit f39bc44 into master Jun 30, 2020
@khmoran khmoran deleted the caseReference branch June 30, 2020 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants